Skip to content

Fix search order in premake.findProjectScript#2678

Open
mercury233 wants to merge 1 commit intopremake:masterfrom
mercury233:patch-include-local
Open

Fix search order in premake.findProjectScript#2678
mercury233 wants to merge 1 commit intopremake:masterfrom
mercury233:patch-include-local

Conversation

@mercury233
Copy link
Copy Markdown
Contributor

What does this PR do?

fix #1783

Current problem:

When include("abc") is called, Premake looks for those files under each directory in premake.path:

  • A file named abc
  • A file named abc.lua
    • This behavior is not mentioned in the documentation
  • Files named premake5.lua and premake4.lua inside an abc subdirectory

premake.path includes the directory of the currently executing script, paths specified via command-line arguments or environment variables, ~/.premake, /usr/local/share/premake, the directory containing the Premake executable, etc.

The concrete search order is: for each candidate filename, search through all these path entries in order, stopping when a match is found.

This creates a problem: entries toward the end of premake.path are supposed to serve as fallback locations, yet the first candidate filename may match against them. For example, include("bitmap") intends to load ./bitmap/premake5.lua, but if the Premake executable lives in /usr/bin and that directory happens to contain a file named bitmap, that file is returned, even though it is a binary executable.

Fix:

Perform a local file-system pass over all candidate filenames first, before delegating to os.locate for path-based search.
Update the documentation to describe the search order and the automatic .lua extension behavior.

How does this PR change Premake's behavior?

Before this change, a file in an external search path whose name matches the first candidate for include("bitmap") would be loaded in preference to a local ./bitmap/premake5.lua. After this change, ./bitmap/premake5.lua is loaded first.
This is unlikely to break existing users: anyone loading a shared lua script from an external path would not normally also have a same-named lua script in the working directory, and would not normally omit the .lua extension.

Anything else we should know?

This fix mitigates the most common manifestation of the problem. The underlying ordering issue remains.

os.isfile is affected by do_chdir in lua_auxlib.c, so it also operates relative to the directory of the currently executing script.

The current implementation intermingles the code paths for searching embedded scripts and file-system scripts, making it harder to maintain and potentially introducing subtle bugs. Further refactoring may be warranted.

Did you check all the boxes?

  • Focus on a single fix or feature; remove any unrelated formatting or code changes
  • Add unit tests showing fix or feature works; all tests pass
  • Mention any related issues (put closes #XXXX in comment to auto-close issue when PR is merged)
  • Follow our coding conventions
  • Minimize the number of commits
  • Align documentation to your changes

You can now support Premake on our OpenCollective. Your contributions help us spend more time responding to requests like these!

Copy link
Copy Markdown
Member

@nickclark2016 nickclark2016 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good at first glance, but I want to let this get more eyes on it and let it stew in my mind for a bit before merging. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Premake5 trying to include /usr/bin/bitmap instead of folder with premake5 script

3 participants